Skip to content

Conversation

@afma-odoo
Copy link
Contributor

@afma-odoo afma-odoo commented Nov 5, 2024

task-4237563
task-4257623
task-4308176
task-4369415

@robodoo
Copy link
Collaborator

robodoo commented Nov 5, 2024

Pull request status dashboard

@afma-odoo afma-odoo requested review from a team and dade-odoo November 5, 2024 12:34
@C3POdoo C3POdoo requested a review from a team November 5, 2024 12:35
@afma-odoo afma-odoo force-pushed the 18.0-accounting-analytic-budget-afma branch from 50c95f3 to 9b6025d Compare November 5, 2024 15:15
@afma-odoo afma-odoo removed request for a team November 7, 2024 13:48
@afma-odoo afma-odoo force-pushed the 18.0-accounting-analytic-budget-afma branch 2 times, most recently from 8c30b6d to 1a6648d Compare November 19, 2024 09:11
Copy link
Contributor

@dade-odoo dade-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @afma-odoo :) Great update for this doc! I have a few comments - feel free to reach out in person tomorrow to look at any of them together or message any time if you have questions. Thanks for all your hard work here and sorry for the delay!

@afma-odoo afma-odoo force-pushed the 18.0-accounting-analytic-budget-afma branch from 1a6648d to 9c7453e Compare December 6, 2024 09:00
@afma-odoo
Copy link
Contributor Author

@dade-odoo Thanks a lot for the review :) Definitely improves the explanations!

@afma-odoo afma-odoo requested a review from dade-odoo December 6, 2024 09:52
Copy link
Contributor

@dade-odoo dade-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay that's it for my comments :) Let's see what the big dogs think

@afma-odoo afma-odoo force-pushed the 18.0-accounting-analytic-budget-afma branch from 9c7453e to 6bbbfd7 Compare December 10, 2024 08:06
@afma-odoo
Copy link
Contributor Author

@dade-odoo Thanks a lot for your help ;)

@afma-odoo afma-odoo requested a review from a team December 10, 2024 08:07
@auva-odoo auva-odoo requested review from auva-odoo and removed request for a team December 12, 2024 07:01
Copy link
Contributor

@auva-odoo auva-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dear @afma-odoo, thank you for your work on this topic. I apologize in advance for my comments that are all over the place, I hope you can make sense of it all. (If not, feel free to reach out, and we can work through it together. 😅) Some general comments:

  • The phrasing “in the xxx status” felt a bit off to me, so I suggested removing the word status entirely (e.g., :guilabel:Open` budgets) or replacing it with with the xxx status. I hope I didn’t miss any, but just in case.

  • FYI, you can just include the app, file name, and anchor name in anchor names so it doesn't get super long (e.g., accounting/budget/analytic-budget) (but honestly it's not that bad here, so if you don't feel like changing them it's ok; just keep it in mind for next time)

  • I was not able to check that the images are compressed (trouble with PyCharm) so could you please double-check that?

PS: Thank you @dade-odoo for the first review 🙂

@afma-odoo afma-odoo force-pushed the 18.0-accounting-analytic-budget-afma branch from 6bbbfd7 to 795fa5f Compare December 12, 2024 15:58
@afma-odoo
Copy link
Contributor Author

@auva-odoo Thanks for the review! Everything was clear to me ;) I confirm the images are compressed!

@afma-odoo afma-odoo requested a review from auva-odoo December 13, 2024 07:54
@afma-odoo afma-odoo force-pushed the 18.0-accounting-analytic-budget-afma branch from 795fa5f to 47f147c Compare December 13, 2024 07:58
Copy link
Contributor

@auva-odoo auva-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @afma-odoo thank you for the changes! I just have one tiny comment so I'll go ahead and approve this already (I would have pushed the change myself but I still have that issue I was talking about the other day 🥺 ). You can r+ when you're ready and all checks have passed 🙂

@robodoo delegate+


To reveal the :guilabel:`Theoretical` amount or percentage, use the :icon:`oi-settings-adjust`
(:guilabel:`adjust settings`) icon in the :guilabel:`Budget Lines`' header. The
:guilabel:`Theoretical` represents the amount of money that could theoretically have been spent or
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
:guilabel:`Theoretical` represents the amount of money that could theoretically have been spent or
:guilabel:`Theoretical` amount represents the amount of money that could theoretically have been spent or

or column? but I feel like something's missing

task-4237563
task-4257623
task-4308176
task-4369415
@afma-odoo afma-odoo force-pushed the 18.0-accounting-analytic-budget-afma branch from 47f147c to 378f878 Compare December 17, 2024 08:37
@afma-odoo
Copy link
Contributor Author

@auva-odoo You're right, thanks! ☺️ and thanks for the approval 🎉

@robodoo r+

robodoo pushed a commit that referenced this pull request Dec 17, 2024
task-4237563
task-4257623
task-4308176
task-4369415

closes #11330

Signed-off-by: Anne-Françoise Marcq (afma) <afma@odoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants